Improve inline breakpoint discovery when expression is multiline. Fix #521#522
Conversation
com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/Breakpoint.java
Show resolved
Hide resolved
| // the lambda doesn't belong to current line at all | ||
| break; | ||
| } | ||
| node = node.getParent(); |
There was a problem hiding this comment.
Recursively checking the parent is not efficient, any way to return earlier?
There was a problem hiding this comment.
Well do you see that we could run forever where theline < sourceLine will be never reached ? Otherwise i think in most cases we will end from the first iteration where line == sourceLine right ? Only in a multiline scenario we will traverse to see if the lambda we found belongs to the same line we add a breakpoint. And I also assume this logic will be executed only for breakpoints that are installed on the source line isn't it ?
There was a problem hiding this comment.
I think we could follow Eclipse’s behavior for breakpoint support on the multiline expression, which allows setting a normal line breakpoint on it directly. This is simpler than the current approach.
I found that we only need a small change on the constructor of BreakpointLocationLocator. We can use the constructor public ValidBreakpointLocationLocator(CompilationUnit compilationUnit, int lineNumber, boolean bindingsResolved, boolean bestMatch, int offset, int end) to initialize the breakpoint validator. This can recognize the multiline lambda on the selected line.
// passing the offset to the constructor, it can recognize the multline lambda expression well
BreakpointLocationLocator locator = new BreakpointLocationLocator(astUnit,
sourceLine, true, true, astUnit.getPosition(sourceLine, 0), 0);
gayanper
force-pushed
the
multi-line-breakpoint-discovery
branch
2 times, most recently
from
c4dfc7d to
f50b519
Compare
gayanper
force-pushed
the
multi-line-breakpoint-discovery
branch
from
f50b519 to
d8845eb
Compare
gayanper
force-pushed
the
multi-line-breakpoint-discovery
branch
from
191b7a5 to
9337cea
Compare
testforstephen
left a comment
testforstephen
left a comment
There was a problem hiding this comment.
LGTM, thanks for contribution.
2 participants
When checking for lambda expression visible in same line, instead of matching the lambda line to current source line, this fix checks if the lambda is related to the current source line as part of a expression by traversing the parents unit we find the source line.